-
Notifications
You must be signed in to change notification settings - Fork 5.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[tune] Fix TrialTerminationReporter in docs #29254
Conversation
Signed-off-by: linusbiostat <[email protected]>
@@ -52,9 +52,11 @@ Extending ``CLIReporter`` lets you control reporting frequency. For example: | |||
|
|||
tuner = tune.Tuner(my_trainable, run_config=air.RunConfig(progress_reporter=ExperimentTerminationReporter())) | |||
results = tuner.fit() | |||
|
|||
|
|||
from ray.tune.trial import Trial |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this import necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes seems to be, just updated it to "ray.tune.experiment.trial"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see where it is used now
Signed-off-by: linusbiostat <[email protected]>
Awesome, thanks - one nit, can we move the Trial import to the top of the code block and leave an empty line after it? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(please see above)
We should also move this to testable doc code, but we can do that in a followup.
Signed-off-by: Antoni Baum <[email protected]>
Co-authored-by: Antoni Baum <[email protected]> Signed-off-by: Weichen Xu <[email protected]>
Signed-off-by: linusbiostat [email protected]
Why are these changes needed?
To make the documentation code work.
TrialTerminationReporter has an init function. Because extending the CliReporter, it's need to take the same init parameters. I've added these with the super() function.
Related issue number
Checks
git commit -s
) in this PR.scripts/format.sh
to lint the changes in this PR.